Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a transitive dependencies endpoint to the REST API #1867

Merged
merged 5 commits into from
Apr 27, 2024

Conversation

mdeicas
Copy link
Collaborator

@mdeicas mdeicas commented Apr 26, 2024

Description of the PR

Adds an endpoint to the REST API to get the transitive dependencies of an artifact or package.

This PR is dependent on and duplicates the changes from #1864 (everything in internal/testing). Once that is merged, I'll rebase this one.

Some design decisions taken were:

  • The graph is traversed using the GraphQL dependencies endpoint
  • We assume that all verbs used in the traversal attach to package version nodes, not package name nodes.
  • Errors that are not actionable to the client are not surfaced. Instead, they are logged and a generic error is returned.

Other things done on this PR are:

  • adding HashEqual and IsOccurrence query to gql client
  • updated the pagination library to take a pointer to the pagination spec, in order to move the checking for nil and applying the default into the library
  • The logger is attached to incoming requests by HTTP middleware, log statements now contain the corresponding HTTP request method and path, and each successful request is logged.

Explanation of the linkCondition parameter

This parameter intends to configure the accuracy of the results. If digest is specified, predicates attached to packages are not used for the graph traversal, because this can lead inconsistencies. For example, suppose there is an artifact A1 that is an occurrence of package P, and there is an SBOM attached to P. It may be misleading to use that SBOM in finding the dependencies of A1, because that SBOM may have been generated for some other artifact A2 -- we don't know.

Asking for accuracy by setting linkCondition = digest requires good data (e.g. SBOMs need to provide the digest of the subject, which I believe Syft does not). In the absence of this, predicates attached to packages can be used for the grpah traversal by setting linkCondition = name.

PR Checklist

  • All commits have a Developer Certificate of Origin (DCO) -- they are generated using -s flag to git commit.
  • All new changes are covered by tests
  • If GraphQL schema is changed, make generate has been run
  • If OpenAPI spec is changed, make generate has been run
  • If collectsub protobuf has been changed, make proto has been run
  • All CI checks are passing (tests and formatting)
  • All dependent PRs have already been merged

Copy link
Collaborator

@jeffmendoza jeffmendoza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really well laid out and high quality. Thanks Marco!

Probably the design decisions and/or algorithm may be tweaked as folks use this and have feedback, but that can be iterated on.

Copy link
Collaborator

@mihaimaruseac mihaimaruseac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, Marco!

Signed-off-by: Marco Deicas <mdeicas@google.com>
Signed-off-by: Marco Deicas <mdeicas@google.com>
Signed-off-by: Marco Deicas <mdeicas@google.com>
Signed-off-by: Marco Deicas <mdeicas@google.com>
Signed-off-by: Marco Deicas <mdeicas@google.com>
@kodiakhq kodiakhq bot merged commit 3bb8b21 into guacsec:main Apr 27, 2024
8 checks passed
@mdeicas mdeicas mentioned this pull request Apr 29, 2024
4 tasks

/********* Implementations of the interface *********/

func (eg byDigest) getDirectDependencies(ctx context.Context, v node) ([]node, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very nice abstractions!

t.Logf("Graphql server shut down")
}
}
t.Cleanup(closeFunc)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

arorasoham9 pushed a commit to arorasoham9/guac that referenced this pull request May 17, 2024
* client testing support

Signed-off-by: Marco Deicas <mdeicas@google.com>

* Add HashEquals and IsOccurrences queries to gql client

Signed-off-by: Marco Deicas <mdeicas@google.com>

* Change Paginate to take pointer to PaginationSpec

Signed-off-by: Marco Deicas <mdeicas@google.com>

* Add better logging

Signed-off-by: Marco Deicas <mdeicas@google.com>

* Implement transitive dependency search

Signed-off-by: Marco Deicas <mdeicas@google.com>

---------

Signed-off-by: Marco Deicas <mdeicas@google.com>
Signed-off-by: Soham Arora <arorasoham9@gmail.com>
@mdeicas mdeicas mentioned this pull request Sep 12, 2024
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants